disaster.model.ts (1)

39-54: Migration SQL is safe and idempotent — minor schema qualification gap.

The DO $$ block with IF NOT EXISTS checks is a solid approach for safe, repeatable migrations. One subtlety: the information_schema.columns query doesn't filter by table_schema, so if multiple schemas contain a disasters table, it could match unintended ones:

-- Current (matches any schema):
WHERE table_name = 'disasters' AND column_name = 'source'

-- Safer:
WHERE table_schema = 'public' AND table_name = 'disasters' AND column_name = 'source'

Low risk in practice if only one schema is in use, but worth hardening.

routes/disasters.protobuf.test.ts (1)

47-73: Explicit source in the test payload is good practice.

The test correctly includes the new field. You could optionally strengthen the assertion to also verify source in the decoded response:

       const decoded = disastersPb.disasters.Disaster.decode(asBuffer(res));
       expect(decoded).toHaveProperty('type', 'fire');
+      expect(decoded).toHaveProperty('source', 'official');

Not required since this test is focused on protobuf content negotiation, but it would improve coverage of the new field end-to-end.

dto/disaster.dto.ts (1)

100-116: DTO mapping logic is solid — one subtle || vs ?? note.

Line 110: raw.source || 'official' uses ||, which means an empty string "" would also fall back to 'official'. This is likely intentional (empty string isn't a valid source), but it's inconsistent with the ?? operator used on lines 111-112 for externalId/sourceUrl. If source should only default when null/undefined (not ""), switch to ??:

-    this.source = raw.source || 'official';
+    this.source = raw.source ?? 'official';

That said, if the validation layer already prevents empty-string sources from reaching here, the current behavior is fine. The distanceKm coercion logic (lines 113-116) is well-handled — guarding against undefined/null before Number() avoids NaN or 0 leaking into responses.

proto/disaster.proto (1)

14-17: Use optional keyword for nullable scalar fields in proto3.

In proto3, scalar fields cannot distinguish "not set" from their default value (empty string "" for strings, 0.0 for double). Since external_id, source_url, and distanceKm are nullable in the model and DTO layers, proto3's default behavior creates ambiguity: distance_km = 0.0 cannot be distinguished from "not computed", and "" cannot be distinguished from genuinely absent for the string fields.

Proto3 supports the optional keyword (protoc v3.15+), which enables explicit presence tracking via has_* methods:

Suggested fix

   string source = 9;
-  string external_id = 10;
-  string source_url = 11;
-  double distance_km = 12;
+  optional string external_id = 10;
+  optional string source_url = 11;
+  optional double distance_km = 12;

graphql/schema.ts (1)

23-26: Consider making source non-nullable in the GraphQL Disaster type.

The database column is NOT NULL DEFAULT 'official' and the DTO defaults to 'official'. Declaring source: String (nullable) in GraphQL is inconsistent. Using source: String! would be more accurate and give clients a stronger contract.

Suggested fix

-    source: String
+    source: String!
     externalId: String
     sourceUrl: String
     distanceKm: Float

services/disaster.service.ts (1)

233-234: SQL concatenation in ST_DWithin / ST_GeomFromText — parameterized but unusual pattern.

Using 'POINT(' || $1 || ' ' || $2 || ')' is safe since $1 and $2 are parameterized values. However, using ST_GeomFromGeoJSON (as done in createDisaster) or ST_MakePoint($1, $2) would be more conventional and avoid the string concatenation inside SQL.